fix: report PR test/build results from a workflow_run job - #200

Addressed Issues:

No filed issue — reporting this from the failing runs themselves. The Test and Build check has been red on every open PR.

Screenshots/Recordings:

Not applicable — CI configuration only, no interface change.

Evidence instead. Step conclusions for run 32115803453 (PR #196):

RequestError [HttpError]: Resource not accessible by integration
  status: 403,
  method: 'POST',
  url: 'https://api.github.com/repos/StabilityNexus/Chainvoice/issues/196/comments'
##[error]Unhandled error: HttpError: Resource not accessible by integration

Same step, same error, on every recent PR run:

Additional Notes:

Cause. A pull_request run triggered from a fork gets a read-only GITHUB_TOKEN regardless of what the workflow's permissions: block requests — that is the point of the fork restriction, and it cannot be granted around. test.yml asked for issues: write and pull-requests: write and then called addLabels and createComment, so those calls were always going to be refused. Contributors work from forks, so this affects all PRs.

The neighbouring Remove test failure label step makes the same forbidden call. It only looks healthy because it swallows the error in a try/catch, so the failure surfaced solely on Report Build Size, which does not.

Handle Test Failure and Handle Build Failure carry the same latent bug: on a fork PR with genuinely broken tests they would 403 and replace the real reason with a permissions error.

Fix. Split the build from the reporting, since only the reporting needs write access.

test.yml drops to contents: read and makes zero API calls. It runs lint / test / build, writes the PR number, the two pass-fail flags, and both logs into a pr-report artifact under if: always(), then fails the job if tests or the build failed. The check now goes red for real reasons only.

pr-report.yml (new) triggers on workflow_run: [completed] for Test and Build. That event runs in the base-repo context with a genuine write token, so it can label and comment. Behaviour is preserved: test case failing and build failed are added when the matching step failed and removed when it did not, and the build-size comment uses the same dist/assets/ filter as before, plus the test log on failure and the build log when the build itself breaks.

Details worth a reviewer's attention:

• The target PR is resolved from the workflow_run payload, never from the artifact. pull_requests[0] covers same-repo PRs; for forks that array is empty, so it falls back to a pulls.list lookup keyed on the run's own head_repository and head_branch, preferring an exact head_sha match. Taking the number from the artifact — as the first push of this branch did — was a privilege escalation, since a fork controls its own copy of test.yml and could have pointed the privileged job at any issue in the repo.
• The surviving artifact fields (tests_failed, build_failed, the two logs) are advisory. A fork can claim its tests passed, but only on its own PR, and the required check's conclusion is what actually gates merge.
• The reporting job never checks out the PR's code. It holds a write token, so executing fork code there would be a privilege-escalation path. It only reads text out of the artifact and quotes it, with backticks replaced so a crafted log cannot break out of the code fence and inject markdown.
• Neither label exists in this repo today (we have bug, documentation, duplicate, enhancement, good first issue, help wanted, invalid, question, wontfix, first-time-contributor, PR has merge conflicts). The script creates the missing one on first use with a colour and description, rather than letting addLabels auto-generate an unstyled label.

Two intentional behaviour changes, flagged because they are visible:

1. The comment is upserted — one comment per PR, found via a hidden <!-- chainvoice-pr-report --> marker and edited in place on each push. The old version posted a fresh comment every run, which stacks up on a PR with several pushes. Say the word if you would rather keep an append-only history.
2. Report Build Size previously ran only on success and printed Metrics not found when the grep missed. Build failures now get the build log quoted instead, which is the more useful artifact when it is the build that broke.

⚠️ Rollout caveat

This PR cannot demonstrate its own fix, for two independent reasons.

1. workflow_run workflows are read only from the default branch, so pr-report.yml stays dormant until this merges to main.
2. test.yml filters on paths: frontend/**, so a workflows-only change does not trigger it at all — Test and Build is absent from this PR's checks rather than green.

Both are expected. Labels and comments start working for every PR, fork or not, once this is on main.

Happy to add .github/workflows/** to that paths filter if you would like CI changes to be self-testing, but it means every workflow edit pays for a full frontend build, so I left it out of this PR.

Not addressed here. The runs also log Node 20 is being deprecated. This workflow is running with Node 24 by default. That concerns the runtime GitHub uses for the action wrappers, not the setup-node version used to build the app, and it was not causing any failure. Bumping checkout / setup-node / github-script to their current major versions belongs in its own PR.

Review round. CodeRabbit raised five findings on the first push and all five are fixed in 045f5668 — the artifact-controlled PR number above, missing concurrency (two quick pushes both created a report), an unbounded comment body (tests and build both failing produced ~120,000 characters against the API's 65,536 cap), marker matching that let a PR author's own comment be overwritten, and unpinned action tags. Every action in both files is now pinned to a commit SHA; the other workflows still use tags, which is a separate cleanup.

One deviation: CodeRabbit also suggested reducing pull-requests: write to read after the lookup. I kept write. The escalation it was guarding against was the arbitrary-issue targeting, which the payload-derived lookup fixes; whether issues: write alone suffices for comments and labels on a pull request is exactly the kind of permission subtlety that caused this bug, and it cannot be tested before merge. Not worth re-creating a 403 to shed one scope — happy to tighten it in a follow-up once the happy path is confirmed on main.

Verification. Both files parse as YAML and the embedded github-script body passes node --check. The comment-budget arithmetic was checked against the worst case CodeRabbit constructed: tests and build both failing with 60,000-character logs now yields a 59,706-character body, and a 500,000-character log yields the same. The workflow_run half cannot be exercised before merge, per the caveat above.

AI Usage Disclosure:

Check one of the checkboxes below:

• This PR does not contain AI-generated code at all.
• This PR contains AI-generated code. I have read the AI Usage Policy and this PR complies with this policy. I have tested the code locally and I am responsible for it.

I have used the following AI models and tools: Claude Code (CLI), model Claude Opus 5

Checklist

• My PR addresses a single issue, fixes a single bug or makes a single improvement.
• My code follows the project's code style and conventions
• If applicable, I have made corresponding changes or additions to the documentation
• If applicable, I have made corresponding changes or additions to tests
• My changes generate no new warnings or errors
• I have joined the Discord server and I will share a link to this PR with the project maintainers there
• I have read the Contribution Guidelines
• Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.
• I have filled this PR template completely and carefully, and I understand that my PR may be closed without review otherwise.